fix(tests): make sure multiple e2e tests run concurrently #1861
Conversation
pull-request-size
bot
added
the
size/M
rubenfonseca
changed the title
Codecov ReportBase: 97.51% // Head: 97.51% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## develop #1861 +/- ##
========================================
Coverage 97.51% 97.51%
========================================
Files 143 143
Lines 6570 6570
Branches 466 466
========================================
Hits 6407 6407
Misses 128 128
Partials 35 35 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
boring-cyborg
bot
added
the
github-actions
rubenfonseca
requested review from
heitorlessa
and removed request for
a team
| running_tasks: List[Future] = [] | ||
| for arn in lambdas_arn: | ||
| # Sleep 0.5, 1, 1.5, ... seconds between each invocation. This way | ||
| # we can guarantee that lambdas are executed in parallel, but they are |
There was a problem hiding this comment.
not sure I follow - how will they be called in parallel if you're adding them to the executor within a 0.5s interval? What was the issue before?
There was a problem hiding this comment.
Use case: we want to fire two lambdas at the same time for the idempotency test. Each Lambda will block for 3 seconds. We want the first lambda to block for 3 seconds, and the second Lambda to throw with "execution is already running".
Before this change, both Lambdas will be fired at the same time, and due to networking jitter and other factors, we couldn't assume the order that they will actually be run. So in this case we guarantee that, while they will be called in parallel, each successive invocation will be slightly shifted in time to make sure we guarantee the output order. Does this make sense?
There was a problem hiding this comment.
It does - thank you!!
For the future (not in this PR), I'd suggest renaming or having a parameter for that purpose so we don't accidentally use it for another use case that might incur side effects in assertion (e.g., execution finishing fast enough).
pull-request-size
bot
added
size/L
|
@heitorlessa addressed your questions and refactored some code, please let me know your thoughts |
boring-cyborg
bot
added
the
dependencies
pull-request-size
bot
added
size/M
rubenfonseca
changed the title
Issue number: #1860
Summary
Changes
Fix E2E tests by marking all tests in the same folder with the same
xdist_group.User experience
Before this change, tests would randomly fail, as a test would try to use infrastructure that was already being teared down.
With this change, all related tests are scheduled together, thus avoiding the problem.
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.